fix(backends): preserve an explicit seed of 0 in sglang and vllm - #11786
fix(backends): preserve an explicit seed of 0 in sglang and vllm#11786pos-ei-don wants to merge 1 commit into
Conversation
mudler#11772 exempted Temperature from the zero-filter in both backend adapters, because proto3 has no field presence and an explicit 0 is indistinguishable from "unset". Seed has exactly the same property and is still filtered: if proto_field != "Temperature" and value in (None, 0, 0.0, [], False, ""): continue A caller pinning `"seed": 0` for a reproducible run therefore gets a random seed instead, with no error and no log line — the one case where the failure is invisible precisely because the request looked deliberate. Both adapters now share a named tuple of fields whose zero is meaningful, so the next one is added in one place rather than as a second special case. Deliberately left filtered: top_k, top_p, min_p and the penalties. Their zero is not a value a caller means — sglang disables top_k with -1, not 0, so forwarding 0 there would turn a default into an invalid argument. Verified on the sglang backend (Qwen3.5-MoE, arm64): with the temperature fix alone, two identical requests at temperature 0 are byte-identical, but pinning seed 0 has no effect until this change. Signed-off-by: pos-ei-don <1822533+pos-ei-don@users.noreply.github.com>
localai-org-maint-bot
left a comment
localai-org-maint-bot
left a comment
There was a problem hiding this comment.
@mudler Good to merge. The change preserves seed 0 in both Python sampling adapters without forwarding zero-valued parameters whose zero means unset or invalid. The upstream Go request builder already randomizes an omitted seed and preserves an explicitly configured 0, so this does not make unseeded requests deterministic. The contributor commit passes DCO; the changed Python files compile cleanly and the diff passes git diff --check.
|
Gentle ping — this has been approved since Aug 30 with no movement since, so I want to make sure it isn't waiting on something from my side. Some context that might help place it: the sibling fix for Happy to rebase, split it, or adjust anything if that makes it easier. |
|
Review pass. Correct, and the zero-value trap I went looking for is not there. Traced the seed chain end to end.
Worth knowing, not blocking: this changes behaviour for anyone driving these backends as standalone gRPC servers without LocalAI. A client that leaves Two process notes. Neither This PR also overlaps #11790 (same sglang |
2 participants
Description
#11772 exempted
Temperaturefrom the zero-filter in both Python backend adapters, because proto3 has no field presence and an explicit0is indistinguishable from "unset".Seedhas exactly the same property and is still filtered:A caller pinning
"seed": 0for a reproducible run therefore gets a random seed instead — no error, no log line. That is the one case where the failure is invisible precisely because the request looked deliberate.Both adapters now share a named tuple of fields whose zero is meaningful, so the next one is added in one place rather than as a second special case.
Notes for Reviewers
Deliberately left filtered:
top_k,top_p,min_pand the penalties. Their zero is not a value a caller means — sglang disables top_k with-1, not0, so forwarding a0there would turn a default into an invalid argument.Verified on an sglang backend in production (Qwen3.5-MoE FP8, GB10/arm64). With the temperature fix from #11772 alone, two identical requests at
temperature: 0are byte-identical; pinningseed: 0still has no effect until this change.For what it is worth as motivation: the silent substitution that #11772 fixed was not neutral in practice. In a code-audit benchmark here (one fixture with a planted bug, 5 runs per temperature, everything else at the model card's
top_p 0.95 / top_k 20), the substituted default landed in the worse band:Anyone benchmarking at
temperature: 0was measuring something other than what they asked for, with no way to tell. A droppedseed: 0has the same shape.Tests extended in both
backend/python/sglang/test.pyandbackend/python/vllm/test.py; they assert that the seed survives and thattop_k/top_pkeep falling through to the engine default.Signed commits